Skip to content

Automatically fold inactive regions - #14796

Open
Colen Garoutte-Carson (Colengms) wants to merge 11 commits into
mainfrom
dev/coleng/auto_fold_inactive_regions
Open

Colen Garoutte-Carson (Colengms) wants to merge 11 commits into
mainfrom
dev/coleng/auto_fold_inactive_regions

Conversation

@Colengms

@Colengms Colen Garoutte-Carson (Colengms) commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds support for folding and unfolding inactive preprocessor regions.

  • Adds the C/C++: Fold All Inactive Regions and C/C++: Unfold All Inactive Regions commands for the active C, C++, or CUDA file.
  • Adds the opt-in C_Cpp.autoFoldInactiveRegions setting to fold inactive regions after IntelliSense finishes processing a newly opened editor.
  • Keeps folding independent of inactive-region colorization, allowing regions to be folded when C_Cpp.dimInactiveRegions is disabled.
  • Leaves existing behavior unchanged by default.

Implementation

Inactive ranges may arrive across multiple IntelliSense result notifications. This change accumulates them per document and waits for a complete IntelliSense pass before folding. This prevents an incomplete result from selecting a nested folding range instead of the surrounding inactive preprocessor branch.

Manual requests made before IntelliSense results are ready remain pending until a complete result arrives. Cached ranges are invalidated when a document changes, opens, or closes. Automatic folding occurs only once per editor instance, including across language-client recovery. Cached inactive-region decorations are refreshed immediately when their appearance settings change, and folding state is routed to the URI-owning client in multi-root workspaces.

Testing

  • Added unit coverage for accumulating complete inactive-region results, replacing and deleting cached results, and producing sorted unique folding lines.
  • Added SingleRootProject scenario coverage for manual fold/unfold, disabled dimming, automatic folding, disabled code folding and enable-later retry, pending requests before IntelliSense is ready, language-client recovery, and empty initial inactive-region results.
  • Added multi-root scenario coverage for URI-based fold/unfold command routing.
  • TypeScript compilation and ESLint pass.
  • Focused unit tests: 5 passing.
  • SingleRootProject scenario tests: 64 passing.
  • MultirootDeadlockTest scenario tests: 12 passing, 1 pre-existing pending test.

Fixes #8992

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

URI-scoped folding state is routed through the wrong client in multi-root workspaces, and one-shot folding state has additional gating issues.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Adds manual and opt-in automatic folding for inactive preprocessor regions.

Changes:

  • Adds inactive-region caching and folding orchestration.
  • Registers the command and setting with localized descriptions.
  • Adds unit and scenario coverage.
File Description
Extension/​src/​LanguageServer/​inactiveRegions.ts Implements inactive-region storage and line selection.
Extension/​src/​LanguageServer/​client.ts Integrates caching, manual folding, and automatic folding.
Extension/​src/​LanguageServer/​extension.ts Registers the folding command handler.
Extension/​src/​LanguageServer/​settings.ts Exposes the automatic-fold setting.
Extension/​package.json Contributes the command, setting, and command-palette condition.
Extension/​package.nls.json Adds user-facing command and setting text.
Extension/​test/​unit/​inactiveRegions.test.ts Tests region accumulation and line generation.
Extension/​test/​scenarios/​SingleRootProject/​tests/​inactiveFolding.test.ts Tests manual and automatic folding scenarios.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Extension/src/LanguageServer/client.ts Outdated
Comment thread Extension/src/LanguageServer/client.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Manual folding can target a stale workspace client and incorrectly suppress later automatic folding.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Previously missed (2)

In code that hasn't changed since last review

Medium severity Manual folds incorrectly suppress later automatic folding

Extension/​src/​LanguageServer/​client.ts:3018

A manual fold also marks the editor as already auto-folded. If the user manually folds, unfolds, and then enables autoFoldInactiveRegions, the settings-change path will refuse to auto-fold this editor even though no automatic fold occurred. Only record the editor when autoFold is actually true.

Medium severity Route document command using the current editor URI

Extension/​src/​LanguageServer/​extension.ts:931

Route this document-scoped command from the current editor URI. ClientCollection.didChangeActiveEditor updates ActiveClient only after awaiting the new client's editor-change notification, so invoking this command during a multi-root editor switch can add the pending fold to the previous client; the actual owner receives the ranges and never fulfills that request.

@Colengms
Colen Garoutte-Carson (Colengms) marked this pull request as ready for review September 24, 2026 22:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation handles result accumulation, lifecycle invalidation, multi-root routing, and configuration behavior with focused coverage.

Review effort: Balanced
Findings: None

@sean-mcmanus Sean McManus (sean-mcmanus) left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is C_Cpp.FoldInactiveRegion needed given that VS Code itself appears to have a whole set of fold/unfold command features.

UPDATE: Oh, well the VS Code commands apply to non-inactive regions too. So, should there be a C_Cpp.UnfoldInactiveRegion command to match what VS Code supports and to undo the Fold command?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Scenario tests leak editor state, and the pending-before-IntelliSense behavior lacks coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Medium severity · 1 Low severity

Open (3)

Comment thread Extension/test/scenarios/MultirootDeadlockTest/tests/inactiveFolding.test.ts Outdated
Comment thread Extension/src/LanguageServer/client.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

A pending manual operation can be consumed by the wrong split-editor view because it is keyed only by document URI.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Track pending folding operations per editor, not document URI

Extension/​src/​LanguageServer/​client.ts:3005

Pending manual operations are keyed only by document URI, although folding state is per TextEditor. If the same document is open in two editor groups and the user requests a fold before results are complete, switching to the other view causes tryApplyInactiveRegionFolding to consume the request there; the original view remains unfolded (and a request from the second view can overwrite the first). Track pending operations by editor instance (for example, a WeakMap<vscode.TextEditor, ...>) and consume only the operation belonging to the active editor.

Comment thread Extension/src/LanguageServer/client.ts Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

An empty initial result leaves automatic folding armed, allowing a later edit to trigger unexpected folding.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Mark empty-result editors as auto-folded

Extension/​src/​LanguageServer/​client.ts:3034

Mark the editor as auto-folded before returning for an empty complete result. Otherwise, a file opened with no inactive branches remains absent from autoFoldedEditors, so a later edit that introduces an inactive branch triggers automatic folding even though automatic folding is documented to happen on open and only once per editor instance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Colengms

Colen Garoutte-Carson (Colengms) commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor Author

(copilot)
Addressed the latest Copilot review in cd241ac. A completed initial pass now marks the editor as auto-folded even when it contains no inactive regions, so a later edit cannot trigger unexpected on-open folding. Added a scenario that opens main.cpp with an empty result, introduces an inactive block, waits for IntelliSense, and verifies it remains unfolded.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Enabling code folding does not retry automatic folding for an already-open eligible editor.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Retry automatic folding when code folding is enabled later

Extension/​src/​LanguageServer/​client.ts:1911

Re-evaluate automatic folding when codeFolding changes too. If a file is opened while autoFoldInactiveRegions is true but code folding is disabled, enabling code folding later leaves the active editor unfolded because the cached complete ranges are never retried (and the outgoing inactive-range flag was already enabled by autoFoldInactiveRegions, so this setting change need not produce another IntelliSense result).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Colengms

Colen Garoutte-Carson (Colengms) commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor Author

(copilot)
Addressed the latest Copilot review in 8c864c4. Changes to C_Cpp.codeFolding now fire the language client folding provider change event before reevaluating automatic folding, so VS Code rebuilds its folding model before editor.fold runs. The regression scenario opens with auto-fold enabled and code folding disabled, verifies the region remains unfolded, enables code folding, and verifies it then folds.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Toggling inactive-region dimming can leave stale decorations in already-open editors.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Refresh inactive-region decorations when dimInactiveRegions changes

Extension/​src/​LanguageServer/​client.ts:1914

With codeFolding enabled (the default), the native dimInactiveRegions flag now remains true even when the user toggles dimming. This settings-change path does not update inactive-region decorations for dimInactiveRegions, so an already-open editor can retain stale dimming (or fail to gain it) until another IntelliSense result happens to arrive. Handle dimInactiveRegions changes here by immediately disposing or rebuilding decorations from the cached ranges while keeping native range reporting enabled independently.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Colengms

Copy link
Copy Markdown
Contributor Author

(copilot-generated)

Addressed the latest Copilot review in c5fd362. Inactive-region decorations are now stored on the URI-owning workspace client and immediately disposed or rebuilt from cached ranges when dimming, opacity, foreground, or background settings change. Decoration types are also disposed when documents close or clients are replaced.

Co-written-by: Copilot

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation and tests consistently cover complete-result accumulation, pending operations, editor lifecycle, configuration changes, and multi-root routing.

Review effort: Balanced
Findings: None

return;
}

this.pendingInactiveRegionFoldingOperations.set(editor.document.uri.toString(), operation);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨Copilot (agent46): [Minor] The earlier Copilot review body raised this, but the current head still queues a manual fold/unfold by document URI rather than by the TextEditor that requested it. If the same file is open in two editor groups, a command queued in view A before inactive ranges are complete can be consumed after focus moves to view B; the editor-scoped editor.fold/editor.unfold command then targets B instead. Could you keep pending requests associated with their originating editor (or cancel on a view change) and cover the split-view case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pull Request

Development

Successfully merging this pull request may close these issues.

Automatically fold inactive regions

3 participants